Skip to content

remove wasm from redzone targets list #22431

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jan 6, 2025
Merged

remove wasm from redzone targets list #22431

merged 1 commit into from
Jan 6, 2025

Conversation

@rdunnington
Copy link
Contributor

@rdunnington rdunnington commented Jan 6, 2025

I ran into a build error on the wasm target where compiling C files with -Werror can fails the build with a warning error: argument unused during compilation: '-mred-zone'. These changes remove wasm from the redzone target list. I'm not sure why wasm was added to the list to begin with in #22190, so maybe this is the wrong change, but based on the repro it seems correct.

It's possible that the red zone option is always present but not configurable for wasm targets but I'm not sure where to look up this info. In that case I'm not sure what the best change would be since the target would technically have a red zone, but zig shouldn't emit flags for it.

Repro steps

// main.c
int get_magic(void)
{
	return 1337;
}
//build.zig
const std = @import("std");

pub fn build(b: *std.Build) void {
    const wasm_target_query: std.Target.Query = .{
        .cpu_arch = std.Target.Cpu.Arch.wasm32,
        .os_tag = std.Target.Os.Tag.freestanding,
    };

    const target = b.resolveTargetQuery(wasm_target_query);
    const optimize = b.standardOptimizeOption(.{});

    var exe = b.addExecutable(.{
        .name = "main",
        .target = target,
        .optimize = optimize,
        .single_threaded = true,
        .link_libc = false,
    });
    exe.addCSourceFiles(.{
        .files = &.{"main.c"},
        .flags = &.{
            "-Wall",
            "-Wextra",
            "-Werror",
        },
    });
    exe.rdynamic = true;
    exe.entry = .disabled;

    b.installArtifact(exe);
}
  1. Save these files into a local dir
  2. zig build
  3. Observe error

My local error repro was:

E:\dev\scratch\zig_redzone_repro> E:\Dev\third_party\zig\build\stage4\bin\zig.exe build
install
└─ install main
   └─ zig build-exe main Debug wasm32-freestanding 1 errors
:1:1: error: argument unused during compilation: '-mred-zone'
error: warning(compilation): failed to delete 'E:\dev\scratch\zig_redzone_repro\.zig-cache\tmp\ed6c7bb464570517-main.o.d': FileNotFound

error: the following command failed with 1 compilation errors:
E:\Dev\third_party\zig\build\stage4\bin\zig.exe build-exe -fno-entry -cflags -Wall -Wextra -Werror -- E:\dev\scratch\zig_redzone_repro\main.c -fsingle-threaded -ODebug -target wasm32-freestanding -mcpu baseline -Mroot --cache-dir E:\dev\scratch\zig_redzone_repro\.zig-cache --global-cache-dir C:\Users\Reuben\AppData\Local\zig --name main -rdynamic --zig-lib-dir E:\Dev\third_party\zig\lib\ --listen=-
Build Summary: 0/3 steps succeeded; 1 failed
install transitive failure
└─ install main transitive failure
   └─ zig build-exe main Debug wasm32-freestanding 1 errors
error: the following build command failed with exit code 1:
E:\dev\scratch\zig_redzone_repro\.zig-cache\o\ff14f003ca7319c829e210acc89e8f97\build.exe E:\Dev\third_party\zig\build\stage4\bin\zig.exe E:\Dev\third_party\zig\lib E:\dev\scratch\zig_redzone_repro E:\dev\scratch\zig_redzone_repro\.zig-cache C:\Users\Reuben\AppData\Local\zig --seed 0x26448b83 -Zcf217a9c049a99c2

@alexrp
Copy link
Member

alexrp commented Jan 6, 2025

It was added because WASM does, in fact, have a red zone in its ABI, and LLVM supports enabling/disabling usage of it. The issue is that the Clang driver doesn't currently process -m(no-)red-zone for WASM: llvm/llvm-project#119997

Removing WASM from the list should be fine until the LLVM 20 upgrade.

@alexrp alexrp enabled auto-merge (rebase) January 6, 2025 16:34
@alexrp alexrp mentioned this pull request Jan 6, 2025
Open
@rdunnington
Copy link
Contributor Author

rdunnington commented Jan 6, 2025

Ok, I see, interesting! Thanks for the quick turnaround.

@alexrp alexrp merged commit f34ef39 into ziglang:master Jan 6, 2025
10 checks passed
@rdunnington rdunnington deleted the fix-wasm-redzone branch January 6, 2025 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alexrp alexrp alexrp approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@rdunnington @alexrp